-
Notifications
You must be signed in to change notification settings - Fork 426
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Convert mod_mam to gen_hook #3841
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Codecov ReportBase: 83.10% // Head: 83.12% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #3841 +/- ##
==========================================
+ Coverage 83.10% 83.12% +0.01%
==========================================
Files 534 534
Lines 34049 34086 +37
==========================================
+ Hits 28297 28333 +36
- Misses 5752 5753 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
This comment was marked as outdated.
This comment was marked as outdated.
781238a
to
b0de2c7
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
d9565e2
to
1be4e62
Compare
This comment was marked as outdated.
This comment was marked as outdated.
1be4e62
to
b4a25be
Compare
This comment was marked as outdated.
This comment was marked as outdated.
b4a25be
to
4a952df
Compare
This comment was marked as outdated.
This comment was marked as outdated.
4a952df
to
3351768
Compare
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few comments from an unfinished review.
57220c8
to
170ee1b
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
170ee1b
to
d4b6026
Compare
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While executable code is perfectly correct, lots of type information are lost or not given. Below a few comments, but same applies to many function specs.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few very hidden typos and forgotten types 😄
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just two more comments 😄
This PR has so many changes, mostly mechanical ones, that it gets quite confusing to review, sorry for not noticing these few more errors before 😅
5782c7c
to
f5402da
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💪🏽 💪🏽
This comment was marked as outdated.
This comment was marked as outdated.
Acc :: any(), | ||
Params :: #{message_count := integer()}, | ||
Acc :: ok, | ||
Params :: map(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also here it is also count := integer()
, no losing the type information better
f5402da
to
af2a2a4
Compare
small_tests_24 / small_tests / af2a2a4 small_tests_25 / small_tests / af2a2a4 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / af2a2a4 graphql_roster_SUITE:admin_roster_http:admin_decline_subscription_ask{error,
{timeout_when_waiting_for_stanza,
[{escalus_client,wait_for_stanza,
[{client,
<<"[email protected]/res1">>,
escalus_tcp,<0.29007.0>,
[{event_manager,<0.29005.0>},
{server,<<"domain.example.com">>},
{username,<<"bOb_admin_decline_subscription_ask_1120">>},
{resource,<<"res1">>}],
[{event_client,
[{event_manager,<0.29005.0>},
{server,<<"domain.example.com">>},
{username,
<<"bOb_admin_decline_subscription_ask_1120">>},
{resource,<<"res1">>}]},
{resource,<<"res1">>},
{username,<<"bob_admin_decline_subscription_ask_1120">>},
{server,<<"domain.example.com">>},
{host,<<"localhost">>},
{port,5222},
{auth,{escalus_auth,auth_plain}},
{wspath,undefined},
{username,<<"bOb_admin_decline_subscription_ask_1120">>},
{server,<<"domain.example.com">>},
{host,<<"localhost">>},
{password,<<"makrolika">>},
{stream_id,<<"e51894e8c0170be0">>}]},
1],
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_client.erl"},
{line,136}]},
{graphql_roster_SUITE,admin_decline_subscription_ask_story,3,
[{file,
"/home/circleci/project/big_tests/tests/graphql_roster_SUITE.erl"},
{line,255}]},
{e... ldap_mnesia_24 / ldap_mnesia / af2a2a4 ldap_mnesia_25 / ldap_mnesia / af2a2a4 dynamic_domains_mysql_redis_25 / mysql_redis / af2a2a4 dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / af2a2a4 dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / af2a2a4 elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / af2a2a4 internal_mnesia_25 / internal_mnesia / af2a2a4 pgsql_mnesia_25 / pgsql_mnesia / af2a2a4 riak_mnesia_24 / riak_mnesia / af2a2a4 mysql_redis_25 / mysql_redis / af2a2a4 mssql_mnesia_25 / odbc_mssql_mnesia / af2a2a4 pep_SUITE:pep_tests:unsubscribe_after_presence_unsubscription{error,
{{badmatch,
[{xmlel,<<"message">>,
[{<<"from">>,
<<"alice_unsubscribe_after_presence_unsubscription_2632@localhost">>},
{<<"to">>,
<<"bob_unsubscribe_after_presence_unsubscription_2632@localhost/res1">>},
{<<"type">>,<<"headline">>}],
[{xmlel,<<"event">>,
[{<<"xmlns">>,
<<"http://jabber.org/protocol/pubsub#event">>}],
[{xmlel,<<"items">>,
[{<<"node">>,<<"tmh4IOYAhpEM8FsngDzsnw==">>}],
[{xmlel,<<"item">>,
[{<<"id">>,<<"salmon">>}],
[{xmlel,<<"entry">>,
[{<<"xmlns">>,
<<"http://www.w3.org/2005/Atom">>}],
[]}]}]}]},
{xmlel,<<"headers">>,
[{<<"xmlns">>,<<"http://jabber.org/protocol/shim">>}],
[]}]}]},
[{pep_SUITE,'-unsubscribe_after_presence_unsubscription/1-fun-0-',2,
[{file,"/home/circleci/project/big_tests/tests/pep_SUITE.erl"},
{line,384}]},
{escalus_story,story,4,
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,72}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1782}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1291}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1223}]}]}} dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / af2a2a4 bosh_SUITE:essential:accept_higher_hold_value{error,
{{assertEqual,
[{module,bosh_SUITE},
{line,251},
{expression,"get_bosh_sessions ( )"},
{expected,[]},
{value,
[{bosh_session,<<"1c478a9cd568834093618bcb7a0bc4774287b440">>,
<8834.6012.0>}]}]},
[{bosh_SUITE,accept_higher_hold_value,1,
[{file,"/home/circleci/project/big_tests/tests/bosh_SUITE.erl"},
{line,251}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1783}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1292}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1224}]}]}} |
Convert mod_mam to gen_hook.